fix(ui): make the destructive red legible in dark mode - #1139
Conversation
The destructive surface was Tailwind red-600 verbatim (chroma 0.245, near the sRGB gamut edge), which vibrates against white and reads generic next to this project's warm palette. It now sits at chroma 0.20 in light and a lighter 0.17 in dark, which keeps unmistakable danger semantics without shouting. Dark mode also faded the fill to 60% opacity, compositing to a dusty red that read closer to disabled than dangerous, on the one control that should look most certain. Attenuation is already this kit's disabled vocabulary, so the dark variant is gone and dark uses the full token, which is tuned lighter for dark backgrounds. Fixes a latent trap alongside: dark --destructive-foreground was a mid red where light is near-white, so any component painting text-destructive-foreground on bg-destructive would have rendered red on red. Nothing hits it today only because the button hardcodes text-white. Applied to both the registry theme and the gallery's preview scope, which is a hand-maintained mirror rather than a generated one.
vivek7405
left a comment
There was a problem hiding this comment.
Reviewed this and it does not hold up. My colour change introduced accessibility regressions, and the review found them before merge.
The headline: removing dark:bg-destructive/60 drops the dark destructive button from 6.48:1 to 3.95:1 with its white label, under the 4.5:1 minimum for 14px text. The fade I called decorative was load-bearing for contrast, because compositing 60% of a light red over a near-black surface produces a much darker effective fill. Three more fall out of the same token change: the dark destructive alert description goes to 3.89:1, the dark dropdown destructive item to 3.62:1 on hover (less legible hovered than at rest), and the badge still carries the /60 I removed from the button, so the same semantic now paints two visibly different reds side by side.
Computing the space properly shows the real problem is structural, not a bad pick. In dark mode --destructive serves two opposing roles:
dark --destructive |
as fill behind white text | as red text on the card |
|---|---|---|
| L 0.54 | 5.52:1 pass | 3.25:1 fail |
| L 0.62 (this PR) | 3.95:1 fail | 4.53:1 pass |
| L 0.70 (before) | 2.88:1 fail | 6.22:1 pass |
There is no single value that satisfies both, which is precisely why the /60 existed: it let one light token double as a darkened fill. I removed the mechanism reconciling the contradiction without replacing it.
The one part that stands up is light mode: oklch(0.52 0.20 25) gives 6.12:1 against white text, up from 4.76:1, and both old values were outside sRGB and being silently clipped by the browser, which substantiates the premise that the red was poorly chosen.
The principled fix is a light-red fill with DARK text in dark mode, which clears both roles comfortably (5.63:1 as a fill at L 0.65, 5.12:1 as text on the card). That needs the button to use text-destructive-foreground instead of its hardcoded text-white, and the dark foreground token to be near-black rather than the near-white I set here, which was wrong in the other direction. That is a real design change across button, badge, alert, and dropdown-menu, so it is not something to slip into a colour tweak unannounced.
Holding this in draft pending that decision.
| // and the data attributes match shadcn exactly. | ||
| destructive: | ||
| 'bg-destructive text-white hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:bg-destructive/60 dark:focus-visible:ring-destructive/40', | ||
| 'bg-destructive text-white hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40', |
There was a problem hiding this comment.
Removing the dark fade takes the destructive button's white label from 6.48:1 to 3.95:1, under AA for 14px text. The 60% composite over a near-black surface was doing contrast work, not just muting the colour. hover:bg-destructive/90 then lands at 4.68:1, so the button is more legible hovered than at rest.
| --destructive: oklch(0.704 0.191 22.216); | ||
| --destructive-foreground: oklch(0.58 0.22 27); | ||
| --destructive: oklch(0.62 0.17 25); | ||
| --destructive-foreground: oklch(0.97 0.01 17); |
There was a problem hiding this comment.
Near-white is the wrong direction for the dark foreground. It lifts red-on-red from 1.65:1 to 3.61:1, still under AA, so the first component to use the token pair as intended still fails. With a light-red fill in dark mode this wants to be near-BLACK, which clears 5.6:1.
The destructive button rendered a faded, dusty red on a near-black surface, reading closer to disabled than to dangerous on the one control that should look most certain. --destructive plays two roles that pull its lightness in opposite directions: a FILL behind --destructive-foreground, and TEXT on a card. In dark mode the text role needs it light, which leaves it too light for white text on the fill. shadcn resolves that with dark:bg-destructive/60, fading the fill until white clears AA, so the opacity was quietly doing contrast work rather than muting a colour. Pair the fill with its own foreground token instead. Dark now runs a full-strength light red with DARK text, light keeps a deeper red with near-white text, and both roles are legible with no compositing trick. The dark --destructive-foreground was also unusable before this (a mid red, so anything honouring the token pair painted red on red); nothing hit it only because button.ts hardcoded text-white. Chroma drops from Tailwind red-600's 0.245, which sits near the sRGB gamut edge and vibrates against white, and the hue shifts marginally off orange so it does not blur into the brand accent. The new test converts the real tokens through OKLab to sRGB and asserts the WCAG ratio for every pairing a shipped component paints, across all seven base colours. A bad hand-edited oklch() triple looks fine in a screenshot and fails only for the people who most need it not to.
The styling recipe's example palette declares fill tokens without their `-foreground` halves, so an app that follows it and then uses a ui class helper gets a filled variant whose text colour is undefined and falls back to whatever it inherits. That was survivable while the destructive variant hardcoded white, and it is not anymore now that the variant honours its token. The example is a good demonstration of why: its red is deep in light and light in dark, so one fixed near-white foreground would read 6.5:1 in light and 1.7:1 in dark. The two halves have to flip together. The skill lives canonically at the repo root and is bundled into the CLI at prepack, so a scaffolded app picks this up with no second edit.
Review found the base-colour contrast test was reading override keys with a `--` prefix that `base-colors.js` does not use, so every lookup missed, the neutral palette was asserted seven times, and the test covered nothing it claimed to. Fixing the lookup immediately failed on a real defect: theme-stone's dark popover is lighter than neutral's, which lifts the destructive menu item's hover tint and drops that text to 4.47:1. Raising the dark token from L 0.70 to 0.72 clears every base with margin (worst case 4.74:1) and improves every other pairing, since the foreground is dark and the tint moves only a fifth as fast as the text. Also cover the two consumers the first version missed. The destructive menu item on its own tint is the tightest pairing in the kit, which is exactly why leaving it uncovered was wrong. Drop the ratios from the button comment rather than correcting them: one was already stale by a point, and a number duplicated from a test is a number that goes wrong the next time a token moves. The comment now points at the test. Add the same skip guard the sibling registry tests use, so the file skips rather than throwing at import where the registry is absent.
|
Round 2 of the review loop landed in 555d822. Two independent reviewers converged on the same defect in my own test, which turned out to be hiding a real one. The base-colour test was vacuous. It read Fixing it failed immediately, on something real. theme-stone's dark popover is lighter than neutral's, which lifts the destructive menu item's hover tint and drops that text to 4.47:1. Raised the dark token from L 0.72 (worst case now 4.74 on stone, 4.89 on neutral). Every other pairing improved too, since the foreground is dark and the tint moves a fifth as fast as the text. Also this round: two uncovered consumers added to the pairings (alert description at The stale ratios in the button comment are deleted rather than corrected. One was already off by more than a point, and a number copied out of a test is a number that goes wrong the next time a token moves; the comment points at the test now. Round 3 is running. Not marking ready until a round comes back clean. |
The previous commit cleared theme-stone's 4.47:1 dropdown-menu item by raising the dark `--destructive` from L 0.70 to 0.72. That worked, but it changed a colour every destructive surface uses to fix a problem that lived in one component's hover tint, and it visibly washed out the destructive button, which is the control the token exists for. Put the red back and take the 0.02 out of the tint instead. The menu item's own red text sits ON that tint, so lowering it from shadcn's dark `/20` to `/15` widens exactly the pairing that was failing and touches nothing else: worst case 4.89:1, against 4.47:1 before. The token is a shared colour and the tint is local to one variant, so the local surface is where a local problem belongs.
Review round 4 caught that moving the fix from the token to a component class string moved it out from under the test. The token is read from themes/index.css, so a bad edit there fails; the tint was a hardcoded 0.15 in the test, so restoring shadcn's /20 passed green while theme- stone dropped back to the 4.47:1 the fix exists to prevent. Same shape as the vacuous base-colour lookup two rounds ago, one surface over. Read the alpha out of dropdown-menu.ts per mode, and require the focus and hover variants to agree, since they paint the same state. The test now measures what ships rather than what it was told ships. Also record the divergence at the call site, which is what the AGENTS.md note added earlier in this PR asks for and this one file was missing.
|
Round 4 landed in 02614c3, plus the tint fix in 84357be. The owner rejected the round-3 fix, correctly. I had cleared theme-stone's 4.47:1 menu item by lightening the dark Round 4 then caught that the fix had escaped its own test. Moving it from the token to a component class string moved it out from under the assertions: the token is read from Two new counterfactuals prove it: restoring PR body corrected to the shipped design, since it still described the reverted L 0.72 version. Browser suite re-run after the tint change: 587 pass on each of Chromium, Firefox, WebKit. Round 5 running. Still not marking ready until a round comes back clean. |
The dark destructive button had drifted into a light coral with dark text. It cleared contrast comfortably and did not read as a destructive button, which is the only thing that control has to do. The cause was accepting a false constraint. --destructive has to be a LIGHT red in dark mode, because the same token paints error text on a near-black card. A fill under white text has to be DARK. The window between those is genuinely empty, so I treated "one token, two roles" as fixed and moved the foreground instead, trading the button's appearance for a metric. Splitting the token is the obvious answer once the constraint is stated out loud: --destructive-fill for the button and badge, --destructive left free for its text role. Dark mode now runs a saturated red under white text, which is what a delete button looks like, and light mode is unchanged because there the two values are the same. Also revert the dropdown-menu tint divergence. theme-stone measured 4.49:1 on that pairing BEFORE any of this work, so it arrived with shadcn rather than being caused here, and fixing it means diverging on a class string. Tracked on its own issue instead, and the test marks that pairing todo rather than deleting it, so the measurement stays visible. Generalise the alpha derivation while here: the hover fill and alert description alphas were hardcoded copies of shipped class strings, the same gap the menu tint had. One Tailwind notch on either is enough to cross AA with the suite still green.
The dark fill was landing deeper than any shipped design system, which made the confirm button read heavy rather than urgent and left it barely separated from the dialog behind it (3.68:1 against --background). That was contrast headroom nobody asked for. Compare the reds actually shipped for a dark destructive button: Radix red-9 sits at 3.75:1 against white, Bootstrap at 4.34, GitHub Primer at 4.41, all below AA. Ours was 5.15, which is not a virtue when the cost is a duller button. L 0.58 is the brightest the fill can go while white text still clears 4.5:1 (4.54). It reads essentially as GitHub's red without the AA miss, and separation from the surface improves from 3.68 to 4.18. The light fill is unchanged. Its surface is white, so it has no such ceiling and no such problem.
|
Closing unmerged. The scope got far out of proportion to the ask, and the underlying premise was wrong: the original dark button measured 6.20:1 and was never an accessibility problem, so most of what this grew into was solving something that did not need solving. Nothing here reached users. Reverting the accordion change separately, and keeping only #1144 (the fixed-header shift) from the follow-ups. Left on the branch |
Closes #1138
The dark destructive button rendered a dusty, desaturated red. The light one was Tailwind
red-600verbatim, whose chroma sits near the sRGB gamut edge and vibrates against white.Why the dark one was dusty
--destructivecarries two roles that want opposite lightness:--destructive-foreground, on the button and badge.--card, in the alert variant,errorClass(), and a destructive menu item.On a near-black card the text role needs a LIGHT red. Under white text the fill needs a DARK one. Measured across the range, the window where one value satisfies both is empty:
--destructiveshadcn resolves this with
dark:bg-destructive/60, fading the light red until white text clears AA. It works, but compositing toward a neutral background desaturates, which is exactly the dusty look.The fix: split the token
--destructive-fillfor the filled variants,--destructiveleft free for its text role. Dark runs a saturated red under white text; light is unchanged, because there both values are the same.--destructiveoklch(0.52 0.20 25)oklch(0.70 0.17 25)--destructive-filloklch(0.52 0.20 25)oklch(0.58 0.20 25)--destructive-foregroundoklch(0.985 0 0)oklch(0.985 0 0)Both new fills are inside the sRGB gamut. Both old tokens were slightly outside it (dark R 1.0127, light G −0.0094).
The dark fill sits at L 0.58 because that is the brightest white text can sit on and still clear 4.5:1. Worth stating explicitly, since a first pass landed at 0.55 and measured 5.15:1, deeper than any shipped system: Radix red-9 is 3.75 against white, Bootstrap 4.34, GitHub Primer 4.41, all below AA. Headroom past the threshold buys nothing and costs saturation, so the fill is tuned to the ceiling rather than away from it.
This is a presentation divergence from shadcn, recorded in
packages/ui/AGENTS.mdinvariant 5. The variant names, size names, and data attributes are untouched.Measured ratios
/90/90on a cardWorst case across all seven base colours: 4.54, the dark fill under its foreground. That is the binding constraint by design, since the fill is tuned right up to the AA threshold.
Tests
packages/ui/test/destructive-contrast.test.jsconverts the real tokens through OKLab to sRGB and asserts WCAG for every pairing above, in both modes, across all seven base colours. Translucent alphas are DERIVED from the shipped class strings rather than restated, because one Tailwind notch is enough to cross AA and a hardcoded copy would not notice.website/test/ssr/ui-gallery.test.tsgains a mirror-drift guard: the gallery's preview palette is a hand-maintained copy of the kit theme, so every shared token is now asserted to match across the light block and both dark blocks.Counterfactuals, each run after committing:
dark: fill behind its foreground is 1.65:1button.ts→the destructive fill must carry --destructive-foregroundthe preview's --destructive has drifted from the kit themethe dark menu item tint alphas disagree with each otherSuites:
@webjsdev/ui191 pass 1 todo, website 129 pass, browser 587 pass on Chromium, Firefox, and WebKit.webjs checkclean. Verified in a real browser in both themes on/ui/buttonand/ui/alert-dialog.One known failure, tracked not hidden
The destructive dropdown-menu item keeps its own red text ON its hover tint, and under theme-stone's lighter popover shadcn's dark
/20puts them at 4.47:1.It is not caused by this change: the pre-existing token measured 4.49:1 on the same pairing, so it arrived with shadcn. Fixing it means diverging on a class string, which is a parity decision that should not ride along with a colour change. Filed as #1143, and the test marks that pairing
todorather than deleting it, so the measurement is reported on every run.Follow-ups filed
examples/blognever redeclares--destructivein light mode while setting--cardto white, so its failed-login and failed-post messages render at 2.91:1. Live, not dormant.position: fixedheader right by half the scrollbar width, because the scroll lock compensates by paddingbodyand a fixed element is laid out against the viewport. Unrelated to colour, found while verifying this.What I got wrong along the way
Recorded because the review rounds are in the commit history and the reasoning is not obvious from the diff.
--prefix thatbase-colors.jsdoes not use, so it asserted the neutral palette seven times and covered nothing.--destructiveglobally. That changed a colour every destructive surface uses in order to fix one component's hover tint, and washed out the button. Reverted.